Skip to content

Fix typing contract for Blob client max_concurrency#45598

Merged
nateprewitt merged 4 commits intoAzure:mainfrom
nateprewitt:max_concurrency_blob
Mar 11, 2026
Merged

Fix typing contract for Blob client max_concurrency#45598
nateprewitt merged 4 commits intoAzure:mainfrom
nateprewitt:max_concurrency_blob

Conversation

@nateprewitt
Copy link
Member

@nateprewitt nateprewitt commented Mar 10, 2026

Description

This is 1 of 3 PRs to address inconsistent typing across the Storage APIs. While working on typing for a project (adlfs) that uses the Azure Python SDK, we found our long standing pattern of passing None when deferring the decision to the SDK doesn't match the type contract on the SDK.

Despite the stub file, the SDK does handle None correctly for the majority of the public blob, file-datalake, and file-share APIs. It's also typed inconsistently with some APIs already being Optional[int], and others only allowing int.

This PR consolidates behavior to be consistent across all Blob APIs and unifies the max_concurrency default concept on a shared DEFAULT_MAX_CONCURRENCY constant. Once we agree on what's required to get this merged, I can replicate to the other clients. For testing, there's not a great way to test this through the public APIs. The changes are very narrow and there are existing tests that already exercise the default path. I can add more though if it's deemed necessary.

I also updated the deprecated methods that use max_concurrency to keep everything consistent. If we'd like to leave those untouched, I can remove those changes.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

Copilot AI review requested due to automatic review settings March 10, 2026 00:44
@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Mar 10, 2026
@nateprewitt
Copy link
Member Author

nateprewitt commented Mar 10, 2026

@microsoft-github-policy-service agree company="Microsoft"

Copilot AI reviewed Mar 10, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the Blob client max_concurrency typing contract with the SDK’s long-standing runtime behavior by allowing None to mean “use the SDK default”, and centralizes that default into a shared constant.

Changes:

  • Introduce DEFAULT_MAX_CONCURRENCY and use it when max_concurrency is None.
  • Update sync/async downloaders and deprecated convenience methods to accept max_concurrency=None.
  • Update sync/async BlobClient stub typings to Optional[int] = None.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
sdk/storage/azure-storage-blob/azure/storage/blob/aio/_download_async.py Accept Optional[int] for max_concurrency and default via DEFAULT_MAX_CONCURRENCY in async downloader paths.
sdk/storage/azure-storage-blob/azure/storage/blob/aio/_blob_client_async.pyi Updates public async stub signatures to Optional[int] = None.
sdk/storage/azure-storage-blob/azure/storage/blob/_shared/constants.py Adds shared DEFAULT_MAX_CONCURRENCY constant.
sdk/storage/azure-storage-blob/azure/storage/blob/_download.py Accept Optional[int] for max_concurrency and default via DEFAULT_MAX_CONCURRENCY in sync downloader paths.
sdk/storage/azure-storage-blob/azure/storage/blob/_blob_client_helpers.py Defaults max_concurrency to the shared constant when omitted/None in option builders.
sdk/storage/azure-storage-blob/azure/storage/blob/_blob_client.pyi Updates public sync stub signatures to Optional[int] = None.
Comments suppressed due to low confidence (2)

sdk/storage/azure-storage-blob/azure/storage/blob/aio/_download_async.py:843

  • The deprecated methods now default max_concurrency to None, but the docstrings still document it as :param int max_concurrency: without mentioning that None is accepted and means “use the default”. Please update the docstrings (and/or type hints) to reflect the new Optional behavior so generated docs match the supported contract.
    async def content_as_bytes(self, max_concurrency=None):
        """DEPRECATED: Download the contents of this file.

        This operation is blocking until all data is downloaded.

        This method is deprecated, use func:`readall` instead.

        :param int max_concurrency:
            The number of parallel connections with which to download.
        :return: The contents of the file as bytes.
        :rtype: bytes
        """
        warnings.warn(
            "content_as_bytes is deprecated, use readall instead",
            DeprecationWarning
        )
        if self._text_mode:
            raise ValueError("Stream has been partially read in text mode. "
                             "content_as_bytes is not supported in text mode.")

        self._max_concurrency = max_concurrency if max_concurrency is not None else DEFAULT_MAX_CONCURRENCY
        return await self.readall()

    async def content_as_text(self, max_concurrency=None, encoding="UTF-8"):
        """DEPRECATED: Download the contents of this blob, and decode as text.

        This operation is blocking until all data is downloaded.

        This method is deprecated, use func:`readall` instead.

        :param int max_concurrency:
            The number of parallel connections with which to download.

sdk/storage/azure-storage-blob/azure/storage/blob/_download.py:900

  • The deprecated methods now default max_concurrency to None, but the docstrings still document it as :param int max_concurrency: without mentioning that None is accepted and means “use the default”. Please update the docstrings (and/or type hints) to reflect the new Optional behavior so generated docs match the supported contract.
    def content_as_bytes(self, max_concurrency=None):
        """DEPRECATED: Download the contents of this file.

        This operation is blocking until all data is downloaded.

        This method is deprecated, use func:`readall` instead.

        :param int max_concurrency:
            The number of parallel connections with which to download.
        :return: The contents of the file as bytes.
        :rtype: bytes
        """
        warnings.warn(
            "content_as_bytes is deprecated, use readall instead",
            DeprecationWarning
        )
        if self._text_mode:
            raise ValueError("Stream has been partially read in text mode. "
                             "content_as_bytes is not supported in text mode.")

        self._max_concurrency = max_concurrency if max_concurrency is not None else DEFAULT_MAX_CONCURRENCY
        return self.readall()

    def content_as_text(self, max_concurrency=None, encoding="UTF-8"):
        """DEPRECATED: Download the contents of this blob, and decode as text.

        This operation is blocking until all data is downloaded.

        This method is deprecated, use func:`readall` instead.

        :param int max_concurrency:
            The number of parallel connections with which to download.

@nateprewitt nateprewitt force-pushed the max_concurrency_blob branch from 8a3543e to 250bea3 Compare March 10, 2026 01:32
@nateprewitt nateprewitt mentioned this pull request Mar 10, 2026
Draft
weirongw23-msft
weirongw23-msft reviewed Mar 10, 2026
View reviewed changes
jalauzon-msft
jalauzon-msft reviewed Mar 10, 2026
View reviewed changes
Copy link
Member

@jalauzon-msft jalauzon-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this! Approach looks good if you were planning to replicate elsewhere.

This was referenced Mar 10, 2026
Merged
Merged
@nateprewitt nateprewitt merged commit 1abb890 into Azure:main Mar 11, 2026
20 checks passed
@nateprewitt nateprewitt deleted the max_concurrency_blob branch March 11, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jalauzon-msft jalauzon-msft jalauzon-msft left review comments

Copilot code review Copilot Copilot left review comments

@weirongw23-msft weirongw23-msft weirongw23-msft approved these changes

@annatisch annatisch Awaiting requested review from annatisch annatisch is a code owner

@vincenttran-msft vincenttran-msft Awaiting requested review from vincenttran-msft vincenttran-msft is a code owner

Assignees

No one assigned

Labels

Storage Storage Service (Queues, Blobs, Files)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nateprewitt @jalauzon-msft @weirongw23-msft